Skip to content

[Test] Add E2E coverage for the language signal - #3507

Open
yu3zhang1 wants to merge 4 commits into
vllm-project:mainfrom
yu3zhang1:test/language-signal-e2e-3178
Open

[Test] Add E2E coverage for the language signal#3507
yu3zhang1 wants to merge 4 commits into
vllm-project:mainfrom
yu3zhang1:test/language-signal-e2e-3178

Conversation

@yu3zhang1

@yu3zhang1 yu3zhang1 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Related #3178

Purpose

Add e2e test for the language routing signal (config.SignalTypeLanguage).
It currently has only Go unit tests for its classifier logic
(language_classifier_test.go) and no end-to-end behavioral coverage
through a deployed request path, as required by #3178.

This PR adds:

  • An es (Spanish) language rule and a spanish_language decision
    (priority 25) to e2e/profiles/ai-gateway/values.yaml.
  • e2e/testcases/language_routing.go, following the existing
    pkgtestcases.Register(...) pattern used by event_routing.go,
    asserting the x-vsr-selected-decision and x-vsr-matched-language
    response headers.
  • e2e/testcases/testdata/language_routing_cases.json with two positive
    cases (distinct Spanish-language queries) and one negative case (an
    English, non-Spanish query).
  • language-routing registered in BaselineRouterContract, a new row in
    signal_routing_contract_test.go, and the ENVOY_AI_GATEWAY_CI_TESTS
    list in .github/workflows/integration-test-k8s.yml — all three
    registration points needed for the test to actually run in CI, added
    up front based on review feedback from [Test] Add E2E coverage for the event signal #3399.

Owning Workgroup: Evaluation & Quality (wg/evaluation-quality).

Test Plan

  • gofmt -l e2e/testcases/language_routing.go
  • go vet ./... (run from e2e/)
  • go build ./... (run from e2e/)
  • golangci-lint run ./testcases/... ./pkg/testmatrix/... (run from e2e/)
  • go test ./testcases/... -run TestProfilesSelectSignalRoutingContracts
  • make e2e-test-specific E2E_TESTS="language-routing"

Test Result

  • gofmt / go vet / go build: pass.
  • golangci-lint: 0 issues in the files this PR touches (pre-existing
    issues in unrelated files under e2e/testcases/ were left untouched).
  • TestProfilesSelectSignalRoutingContracts: pass, confirms the
    envoy-ai-gateway profile selects language-routing.
  • make e2e-test-specific: not run against a live cluster locally
    (same local Kubernetes/network constraints noted on [Test] Add E2E coverage for the event signal #3399). Deferring
    final e2e confirmation to CI on this PR.
  • Manually verified the priority ordering in values.yaml: the new
    spanish_language decision (priority 25) sits between thinking_decision
    (20) and urgent_request (30), and its Spanish-language test queries
    don't contain any of the other decisions' trigger keywords, so the new
    test cases can't be shadowed by an unrelated decision.

AI assistance (Claude Code) was used to draft this change, under my review;
I read and understand the diff and take responsibility for it.

🤖 Generated with Claude Code

@netlify

netlify Bot commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit eb3a159
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/6a9fbfaa27c49a00085f2238
😎 Deploy Preview https://deploy-preview-3507--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added pr/needs-review Ready for reviewer attention. wg/evaluation-quality Owned by the Evaluation and Quality Workgroup. pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-review Ready for reviewer attention. labels Sep 5, 2026
@yu3zhang1

yu3zhang1 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

E2E (envoy-ai-gateway) runs successfully:

LANGUAGE ROUTING TEST RESULTS
Total Tests: 3
Correct: 3 (100.00%)
[Test] Language routing test completed: 3/3 correct (100.00% accuracy)

@yu3zhang1
yu3zhang1 force-pushed the test/language-signal-e2e-3178 branch from 5d810e7 to ae64a0e Compare September 5, 2026 17:38
@github-actions github-actions Bot added pr/needs-review Ready for reviewer attention. pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-rebase Needs rebase or conflict resolution. pr/needs-review Ready for reviewer attention. labels Sep 5, 2026
drivebyer
drivebyer reviewed Sep 7, 2026
View reviewed changes

@drivebyer drivebyer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocker: the load/result/print scaffolding here mirrors event_routing.go almost verbatim. Could you extract a shared helper in this PR and reuse it in both cases?

@yu3zhang1
yu3zhang1 force-pushed the test/language-signal-e2e-3178 branch from 5988a27 to f079dd4 Compare September 7, 2026 15:47
yu3zhang1 added a commit to yu3zhang1/semantic-router that referenced this pull request Sep 7, 2026
@yu3zhang1 @claude
Extract shared signal-routing test harness for event and language
ee699b0 
Addresses review feedback from @drivebyer on vllm-project#3507: event_routing.go
and language_routing.go duplicated the same load/loop/print
scaffolding almost verbatim.

* Extract it into `signal_routing_helpers.go` and reuse it from both,
* Unifying the testdata field name to `expected_matched_signal`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: pikachu <4401183+yu3zhang1@users.noreply.github.com>
@github-actions github-actions Bot added pr/blocked Blocked on a named decision, dependency, or required check. and removed pr/needs-rebase Needs rebase or conflict resolution. labels Sep 7, 2026
@github-actions github-actions Bot added pr/needs-rebase Needs rebase or conflict resolution. and removed pr/blocked Blocked on a named decision, dependency, or required check. labels Sep 7, 2026
@yu3zhang1

yu3zhang1 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Non-blocker: the load/result/print scaffolding here mirrors event_routing.go almost verbatim. Could you extract a shared helper in this PR and reuse it in both cases?

Done. Move the shared code code to signal_routing_helpers.go. PTAL, thanks!

@Xunzhuo Xunzhuo mentioned this pull request Sep 8, 2026
Open
drivebyer
drivebyer previously approved these changes Sep 8, 2026
View reviewed changes

@drivebyer drivebyer left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @yu3zhang1 Please resolve the conflict.

yu3zhang1 and others added 3 commits September 7, 2026 20:41
@yu3zhang1 @claude
[Test] Add E2E coverage for the language signal.
63743fc 
* Add an `es`(Spanish) language rule and a `spanish_language` decision
to the ai-gateway profile.

* Add a new language-routing test case containing two positive cases
(distinct Spanish-language queries) and one negative case (an English, non-Spanish query).

* Register language-routing in BaselineRouterContract, the
signal-routing contract test, and the envoy-ai-gateway CI test list.

Second bounded slice of the signal/projection E2E coverage requested
in vllm-project#3178 (Evaluation & Quality workgroup); remaining signals will
land as separate follow-up slices.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: pikachu <4401183+yu3zhang1@users.noreply.github.com>
@yu3zhang1 @claude
Extract shared signal-routing test harness for event and language
d569927 
Addresses review feedback from @drivebyer on vllm-project#3507: event_routing.go
and language_routing.go duplicated the same load/loop/print
scaffolding almost verbatim.

* Extract it into `signal_routing_helpers.go` and reuse it from both,
* Unifying the testdata field name to `expected_matched_signal`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: pikachu <4401183+yu3zhang1@users.noreply.github.com>
@yu3zhang1 @claude
Fix gci import grouping in signal-routing test files
452a35e 
CI's Full pre-commit checks job failed: gci wants a separate import
group for github.com/vllm-project/semantic-router-prefixed imports,
after the standard-library and third-party groups. Reorder imports in
event_routing.go, language_routing.go, signal_routing_helpers.go, and
signal_routing_contract_test.go to match.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: pikachu <4401183+yu3zhang1@users.noreply.github.com>
@yu3zhang1
yu3zhang1 force-pushed the test/language-signal-e2e-3178 branch from 5c3e4d6 to 452a35e Compare September 8, 2026 03:42
@github-actions github-actions Bot added pr/needs-review Ready for reviewer attention. and removed pr/needs-rebase Needs rebase or conflict resolution. labels Sep 8, 2026
@yu3zhang1

yu3zhang1 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@drivebyer Resolved the conflict, which dismissed your approval. CI is green again — could you re-review when you get a chance? Thanks!

@github-actions github-actions Bot added pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-review Ready for reviewer attention. labels Sep 8, 2026
FAUST-BENCHOU
FAUST-BENCHOU approved these changes Sep 8, 2026
View reviewed changes

@FAUST-BENCHOU FAUST-BENCHOU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thnaks

@mergify

mergify Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

wilsonwu
wilsonwu approved these changes Sep 8, 2026
View reviewed changes

@wilsonwu wilsonwu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify

mergify Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-09-08 07:56 UTC · Rule: default · triggered by rule queue approved pull requests
  • 🟠 Checks running · in-place · dashboard
  • ⏳ Merge · ETA: 2026-09-08 08:51 UTC 🚀
Waiting for
  • any of:
    • check-skipped = PR Gate
    • check-success = PR Gate
  • any of: [🛡 GitHub branch protection]
    • check-neutral = PR Gate
    • check-skipped = PR Gate
    • check-success = PR Gate
All merge conditions
Waiting for
  • any of:
    • check-skipped = PR Gate
    • check-success = PR Gate
  • any of: [🛡 GitHub branch protection]
    • check-neutral = PR Gate
    • check-skipped = PR Gate
    • check-success = PR Gate
All queue conditions
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule default]:
      • any of:
        • check-skipped = PR Gate
        • check-success = PR Gate
      • any of [🛡 GitHub branch protection]:
        • check-neutral = PR Gate
        • check-skipped = PR Gate
        • check-success = PR Gate
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • base = main
      • github-review-approved [🛡 GitHub branch protection]
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = @mergify/Configuration changed
    • check-success = @mergify/Configuration has been deleted
  • any of [📌 queue requirement]:
    • check-neutral = @mergify/Mergify Merge Protections
    • check-skipped = @mergify/Mergify Merge Protections
    • check-success = @mergify/Mergify Merge Protections

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wilsonwu wilsonwu wilsonwu approved these changes
@FAUST-BENCHOU FAUST-BENCHOU FAUST-BENCHOU approved these changes
@drivebyer drivebyer drivebyer left review comments
@shraderdm shraderdm Awaiting requested review from shraderdm shraderdm is a code owner
@ramkrishs ramkrishs Awaiting requested review from ramkrishs ramkrishs is a code owner
@WUKUNTAI-0211 WUKUNTAI-0211 Awaiting requested review from WUKUNTAI-0211 WUKUNTAI-0211 is a code owner
@AayushSaini101 AayushSaini101 Awaiting requested review from AayushSaini101 AayushSaini101 is a code owner
@theohsiung theohsiung Awaiting requested review from theohsiung theohsiung is a code owner
@rootfs rootfs Awaiting requested review from rootfs rootfs is a code owner
@Xunzhuo Xunzhuo Awaiting requested review from Xunzhuo Xunzhuo is a code owner

Assignees

No one assigned

Labels

pr/needs-rebase Needs rebase or conflict resolution. wg/evaluation-quality Owned by the Evaluation and Quality Workgroup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yu3zhang1 @wilsonwu @drivebyer @FAUST-BENCHOU